Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Display timestamp, useful to find timing problem #680

Merged
merged 2 commits into from
Oct 8, 2024

Conversation

davidqge
Copy link
Contributor

I found timestamp in defmt documents, but following their instruction still don't show timestamp.

Spent a week figured out it's because espflash didn't display it.

This change made following work too

in Cargo.toml dependency:
embassy-time ={ version = "0.3.1" , features = ["defmt-timestamp-uptime"] }

Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this show the log level and line? Also, what is the format when not using embassy and the selected feature for timestamps?

@davidqge
Copy link
Contributor Author

davidqge commented Sep 17, 2024

Does this show the log level and line?

Yes, it shows the log level and line too. Using same display code from defmt.

Also, what is the format when not using embassy and the selected feature for timestamps?

Display rest of info without timestamp.
You can also manually add timestamp according to defmt doc.

@SergioGasquez
Copy link
Member

Did some testing in the PR and changes the defmt format from:
image
To:
image

IMHO, the new format lacks of a separator between the level and the message. Is that the default dfmt format that other platform are using?

@davidqge
Copy link
Contributor Author

IMHO, the new format lacks of a separator between the level and the message. Is that the default dfmt format that other platform are using?

Yes, here is an image of defmt
You can customize output if you want to, I just like to get timestamp to debug timing problems.

Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Mind resolving the conflicts?

Copy link
Member

@jessebraham jessebraham left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've resolved the conflict, changes seem fine to me. @SergioGasquez will give you final say on this and let you merge it if you approve :)

Copy link
Member

@SergioGasquez SergioGasquez left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the contribution! And thanks for resolving the conflict @jessebraham!

@SergioGasquez SergioGasquez added this pull request to the merge queue Oct 8, 2024
Merged via the queue into esp-rs:main with commit 30300c4 Oct 8, 2024
28 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants